Skip to content

DOCSP-46269: atlas search & atlas vector search pages#3255

Merged
rustagir merged 22 commits intomongodb:5.xfrom
rustagir:DOCSP-46269-atlas-search
Feb 10, 2025
Merged

DOCSP-46269: atlas search & atlas vector search pages#3255
rustagir merged 22 commits intomongodb:5.xfrom
rustagir:DOCSP-46269-atlas-search

Conversation

@rustagir
Copy link
Copy Markdown
Contributor

@rustagir rustagir commented Jan 28, 2025

@rustagir rustagir requested a review from a team as a code owner January 28, 2025 21:16
@rustagir rustagir requested a review from norareidy January 28, 2025 21:16
@github-actions github-actions Bot added the docs label Jan 28, 2025
@rustagir rustagir marked this pull request as draft January 28, 2025 21:26
@rustagir rustagir force-pushed the DOCSP-46269-atlas-search branch from a22decf to 403d10a Compare January 29, 2025 14:54
Comment thread docs/fundamentals/as-avs.txt Outdated
Comment thread docs/fundamentals/as-avs.txt Outdated
@rustagir rustagir marked this pull request as ready for review January 29, 2025 15:34
Comment thread docs/sessions.txt
Copy link
Copy Markdown
Contributor

@lindseymoore lindseymoore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work so far! A few formatting and wording suggestions

Comment thread docs/fundamentals/as-avs.txt Outdated
Comment thread docs/fundamentals/as-avs.txt Outdated
Comment thread docs/fundamentals/as-avs.txt Outdated
Comment thread docs/fundamentals/as-avs.txt Outdated
Comment thread docs/fundamentals/as-avs.txt Outdated
Comment thread docs/fundamentals/as-avs.txt Outdated
Comment on lines +251 to +253
- ``exact``: ``bool`` (default: ``false``)
- ``filter``: ``QueryInterface`` or ``array`` (default: no filtering)
- ``numCandidates``: ``int`` or ``null`` (default: ``null``)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

S: Same as above. I think I would create a table here with descriptions

Comment thread docs/fundamentals/as-avs.txt Outdated
Comment thread docs/includes/fundamentals/as-avs/AtlasSearchTest.php Outdated
Comment thread docs/includes/fundamentals/as-avs/AtlasSearchTest.php Outdated
Comment thread docs/fundamentals/as-avs.txt Outdated
@rustagir rustagir requested a review from lindseymoore January 29, 2025 20:24
Comment thread docs/fundamentals/as-avs.txt Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be more readable to use a full name instead of this abbreviations I've never seen before?
atlas-search.txt

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to be comprehensive as Atlas Search and Atlas Vector Search are two separate features. But I can change the file name to atlas-search-vector-search

Copy link
Copy Markdown
Member

@GromNaN GromNaN Jan 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe create 2 pages that link each other. Indeed that's 2 distinct features.

Copy link
Copy Markdown
Contributor

@lindseymoore lindseymoore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM w/ small suggestions!

Comment thread docs/fundamentals/as-avs.txt Outdated
Comment thread docs/fundamentals/as-avs.txt Outdated
Comment thread docs/fundamentals/as-avs.txt Outdated
Comment on lines +116 to +125
- ``index``: ``string``
- ``highlight``: ``array``
- ``concurrent``: ``bool``
- ``count``: ``string``
- ``searchAfter``: ``string``
- ``searchBefore`` ``string``
- ``scoreDetails``: ``bool``
- ``sort``: ``array``
- ``returnStoredSource``: ``bool``
- ``tracking``: ``array``
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, perhaps listing the names of the parameters without the data type would suffice then, since the atlas documentation also includes the data type. The data type seems like unnecessary info if I don't know what the parameter is for. I'll leave up to you though!

Comment thread docs/fundamentals/as-avs.txt Outdated
@rustagir rustagir requested a review from GromNaN January 30, 2025 21:25
@rustagir rustagir changed the title DOCSP-46269: as & avs DOCSP-46269: atlas search & atlas vector search pages Jan 31, 2025
Comment thread docs/includes/fundamentals/as-avs/AtlasSearchTest.php Outdated
Comment thread docs/includes/fundamentals/as-avs/AtlasSearchTest.php Outdated
Comment thread docs/fundamentals/vector-search.txt Outdated
@rustagir rustagir requested a review from GromNaN February 7, 2025 20:08
Copy link
Copy Markdown
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 remaining TODO, otherwise LGTM

Comment thread docs/fundamentals/vector-search.txt
@rustagir rustagir merged commit 9fdfbe5 into mongodb:5.x Feb 10, 2025
Copy link
Copy Markdown
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While running tests on the project, I found some issues with those introduced by this PR. They were not executed because the phpunit group is missing. Fixes in #3279

* @runInSeparateProcess
* @preserveGlobalState disabled
*/
public function autocompleteSearchTest(): void
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is not executed. In PHPUnit, test must be prefixed with test: testAutocompleteSearch

* @runInSeparateProcess
* @preserveGlobalState disabled
*/
public function vectorSearchTest(): void
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue with this test name: testVectorSearch

do {
$ready = true;
usleep(10_000);
foreach ($collection->listSearchIndexes() as $index) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get this error:

Undefined variable $collection

The variable is named $moviesCollection.

*/
public function vectorSearchTest(): void
{
$results = Book::vectorSearch(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The class is not imported, add use MongoDB\Laravel\Tests\Models\Book to the file.

$movies = Movie::search(
sort: ['title' => 1],
operator: Search::text('title', 'dream'),
)->get();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Movie::search returns an instance of Illuminate\Support\Collection. The method all() should be called to get all the results. get is to get a single item.

public function autocompleteSearchTest(): void
{
// start-auto-query
$movies = Movie::autocomplete('title', 'jak')->get();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Movie::search returns an instance of Illuminate\Support\Collection. The method all() should be called to get all the results. get is to get a single item.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants